Skip to content

Exit with status 1 on Cobra problems like a missing required flag - #363

Merged
brandur merged 1 commit into
masterfrom
brandur-exit-1-on-cobra-problems
May 20, 2024
Merged

Exit with status 1 on Cobra problems like a missing required flag#363
brandur merged 1 commit into
masterfrom
brandur-exit-1-on-cobra-problems

Conversation

@brandur

@brandur brandur commented May 19, 2024

Copy link
Copy Markdown
Contributor

Here, exit with status 1 from the River CLI when a problem intrinsic to
Cobra occurred like the user entered an unknown command, or didn't pass
required flag.

Cobra is a little odd in its behavior in that it already prints such a
problem to the terminal, but also returns an error with the same
information. This change looks for the presence of an error and exits
with status 1 if one occurred, but doesn't double print any additional
information about it.

Fixes #362.

Here, exit with status 1 from the River CLI when a problem intrinsic to
Cobra occurred like the user entered an unknown command, or didn't pass
required flag.

Cobra is a little odd in its behavior in that it already prints such a
problem to the terminal, but also returns an error with the same
information. This change looks for the presence of an error and exits
with status 1 if one occurred, but doesn't double print any additional
information about it.

Fixes #362.
@brandur
brandur force-pushed the brandur-exit-1-on-cobra-problems branch from dd6f26f to 107fa33 Compare May 19, 2024 06:46
@brandur
brandur requested a review from bgentry May 19, 2024 06:51
Comment thread .github/workflows/ci.yaml
if ./river not-a-command; then
echo "expected non-zero exit code" && exit 1
fi
working-directory: ./cmd/river

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally as well:

$ go run . migrate-up
Error: required flag(s) "database-url" not set
Usage:
  river migrate-up [flags]

Flags:
      --database-url postgres://...   URL of the database to migrate (should look like postgres://...
      --dry-run                       print information on migrations, but don't apply them
  -h, --help                          help for migrate-up
      --max-steps int                 maximum number of steps to migrate
      --show-sql                      show SQL of each migration
      --target-version int            target version to migrate to (final state includes this version, but none after it)

Global Flags:
      --debug     output maximum logging verbosity (debug level)
  -v, --verbose   output additional logging verbosity (info level)

exit status 1

$ echo $?
1

@brandur

brandur commented May 20, 2024

Copy link
Copy Markdown
Contributor Author

Thanks!

@brandur
brandur merged commit 8264a24 into master May 20, 2024
@brandur
brandur deleted the brandur-exit-1-on-cobra-problems branch May 20, 2024 07:19
brandur added a commit that referenced this pull request May 20, 2024
Prepare version 0.6.1 for release, including the changes from #350 (no
premature rescue for jobs with long custom timeouts), #363 (exit with
status 1 in case of bad command/flags) in CLI, and #364 (fix migration
version 4 to be re-runnable).
@brandur brandur mentioned this pull request May 20, 2024
brandur added a commit that referenced this pull request May 21, 2024
Prepare version 0.6.1 for release, including the changes from #350 (no
premature rescue for jobs with long custom timeouts), #363 (exit with
status 1 in case of bad command/flags) in CLI, and #364 (fix migration
version 4 to be re-runnable).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cmd/river: Exit code 0 on flag error

2 participants